Skip to content

Comments

Fix OCSP.verify_response() to check cert_status#14

Open
lnagel wants to merge 1 commit intothorgate:mainfrom
namespace-ee:lenno/fix-ocsp-verify-response
Open

Fix OCSP.verify_response() to check cert_status#14
lnagel wants to merge 1 commit intothorgate:mainfrom
namespace-ee:lenno/fix-ocsp-verify-response

Conversation

@lnagel
Copy link

@lnagel lnagel commented Jan 30, 2026

Summary

  • Add cert_status check to verify_response() method to properly validate certificate validity
  • Add OCSPCertificateRevokedError exception for revoked certificates
  • Add OCSPCertificateUnknownError exception for unknown certificate status
  • Add tests for revoked, unknown, and good certificate statuses

Previously, verify_response() only checked response_status (whether the OCSP server processed the request) but not cert_status (whether the certificate is actually valid). This meant revoked and unknown certificates silently passed validation.

Fixes #13

Test plan

  • Run pytest pyasice/tests/test_ocsp.py -v - all 7 tests pass
  • Run full test suite pytest -v - all 45 tests pass

🤖 Generated with Claude Code

Previously, verify_response() only checked response_status (whether the
OCSP server processed the request) but not cert_status (whether the
certificate is actually valid). This meant revoked and unknown
certificates silently passed validation.

Now raises OCSPCertificateRevokedError for revoked certificates and
OCSPCertificateUnknownError for unknown status.

Fixes thorgate#13

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@JohanViirok
Copy link

Hi @lnagel, thanks for opening this PR and for the detailed issue writeup in #13!

This looks like a legitimate gap in the validation logic, we'll take a closer look at the changes and the test coverage.
Before we dive into the review, it would be helpful to understand the context a bit better: could you explain in more detail how you came across this problem? Was this something you hit in production, or did you discover it through a code review / audit? Also, we're curious how you and your team are using pyasice. What's your use case and setup? Understanding that would help us evaluate the impact and make sure the fix works well for the scenarios people actually rely on.

We'll get back to you with a proper review soon. Thanks again for the contribution!

@JohanViirok JohanViirok requested a review from Jyrno42 February 20, 2026 11:31
@lnagel
Copy link
Author

lnagel commented Feb 20, 2026

could you explain in more detail how you came across this problem? Was this something you hit in production, or did you discover it through a code review / audit?

Our customers have been sporadically experiencing situations where the signing process succeeds normally, but later when opening the .asice containers, the signatures render as "invalid" -- legally not binding. We have been tracing this for a while now and performed a security and compliance audit to pyasice. We covered several gaps in the implementation vs. required standards. This bugfix here is for the case where an incorrect OCSP server was consulted and pyasice considered its response a success, when in reality, that server replied with "certificate status unknown". In reality, for newer Smart ID certificates, a different server must be consulted.

I filed the results of that audit into GitHub issue tickets in case they might be of use.

@lnagel
Copy link
Author

lnagel commented Feb 20, 2026

Also, we're curious how you and your team are using pyasice. What's your use case and setup? Understanding that would help us evaluate the impact and make sure the fix works well for the scenarios people actually rely on.

We currently have a digital signing service in production with pyasice deployed on a public cloud. Basically this uses all the big three signing methods: smart-id, mobile-id and smartcards. This was built around pyasice as it was very convenient for handling the container files directly from an async Python context.

After review of the compliance audit results, consulting with the customer and within the team, we decided ultimately to switch to using libdigidocpp and libcdoc directly from python.

For this purpose we are publishing and maintaining now the Python bindings for both libraries:

Sources for the bindings are available in GitHub at:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OCSP.verify_response() does not validate certificate status

2 participants